Close the Slack gate's flag-repetition bypass, and stop a NUL byte from silencing the bot - #72

Two defects in grapharc/slack/command.py, the module that decides what text from a Slack workspace may become an argv. Same file, same class of mistake — a gate that reads one thing while the CLI runs another, and a refusal path that raises instead of replying.

#61 — repeating --registry walked past the agent opt-in

The gate admits a command by reading a flag's value; argparse's store action then runs the last occurrence, and _flag_value (command.py:331) returned the first. So the two read different values out of the same command line:

kw = dict(workdir=ws, allow_model=True, allow_agent=False)   # agent NOT allowed

parse_command("plan 'ship it' --model openrouter/x/y "
              "--registry grapharc.stdlib:build_registry", **kw)
# SlackCommandError: the stdlib plan registry runs agent tools on the host …   [correct]

parse_command("plan 'ship it' --model openrouter/x/y "
              "--registry grapharc.examples.plan_docs:build_registry "
              "--registry grapharc.stdlib:build_registry", **kw)
# ADMITTED — registries: ['…plan_docs:build_registry', 'grapharc.stdlib:build_registry']
#            --approve injected? False

Judged against the demo registry, which needs no opt-in; executed against grapharc.stdlib:build_registry, which builds agent kinds that run tools on the host under an executor that, in its own words, "confines nothing". GRAPHARC_SLACK_ALLOW_AGENT was never consulted, and the --approve injection that would have put a human in front of the run was skipped in the same step, so nothing downstream caught it either. The exploit is the second --registry: no privilege, no special knowledge.

Fix — the issue's second option, fail-closed: a repeated flag is refused outright. That retires the whole first-vs-last family rather than the one flag that exposed it; a Slack command has no legitimate reason to pass --registry or --model twice, and a gate that must pick which of two occurrences to believe is a gate that can be wrong. The carve-out is a new CommandSpec.repeatable_flags — the options the CLI itself accumulates (agent --allow/--deny, argparse action="append"), where every occurrence reaches the run and there is no other value to diverge from. A duplicated --model falls to the same rule, opted in or not, so it cannot smuggle a backend past the spend gate either.

_flag_value now reads the last occurrence regardless — the cheap half of the belt. With repeats refused there is only ever one, but a future caller assembling an argv some other way should not be able to reopen the gap.

After:

#61 single:      REFUSED: the stdlib plan registry runs agent tools on the host and is off …
#61 dup:         REFUSED: `--registry` was given twice; from Slack a flag may appear only once,
                 because the gate and the CLI would not necessarily read the same one
#61 dup --model: REFUSED: `--model` was given twice; …

#64 — a NUL byte came back as silence

Path(raw).resolve() (command.py:171) raises ValueError, and handle_text_live catches only SlackCommandError, so handle_text("trace a\x00b", cfg) escaped the bolt listener as an unhandled exception and the requester got no reply at all — the one answer a chat bot must never give, since it is indistinguishable from the bot being down.

A NUL anywhere in the request is a refusal now, in the same voice the core tools already use (workspace.py: "cannot name a file"), and _confined turns any ValueError/OSError out of the filesystem into a refusal too, keeping the guarantee for callers of its own.

Folded in from the same issue: the flag allowlist tested token.startswith("--") (command.py:222), so a single-dash token slipped it and was spent as a positional. The allowlist is meant to be exhaustive; any leading dash is a flag now, and one not on the list is refused like any other.

Before / after:

handle_text("trace a\x00b", cfg)  ->  ValueError: lstat: embedded null character in path   [uncaught]
handle_text("trace a\x00b", cfg)  ->  'that contains a NUL byte, which cannot name a file or a command'

parse_command("trace -h", …)      ->  ['trace', '-h']                                       [admitted]
parse_command("trace -h", …)      ->  SlackCommandError: `-h` is not allowed on `trace` from Slack

Tests

Six added to tests/test_slack_gateway.py; the five that cover the defects were confirmed failing on the pre-fix command.py (stashed) and passing after.

• test_a_repeated_registry_cannot_walk_the_agent_opt_in — the issue's repro, in both orders, so neither "the gate reads the last" nor "the gate reads the first" can pass it again; asserts the single-flag refusal and the single-flag admission are unchanged.
• test_a_repeated_model_cannot_smuggle_a_backend_past_the_spend_gate — opted in and not, and the --model x --model=y mixed spelling.
• test_every_gated_flag_is_refused_in_its_duplicated_form — sweeps ALLOWED_COMMANDS and asserts the duplicated form of every admitted, non-repeatable flag, so a gate added later inherits the property instead of having to remember it (the issue's "add a test that every gated flag is checked in its duplicated form").
• test_the_flags_the_cli_accumulates_stay_repeatable — agent --allow/--deny still accumulate.
• test_a_nul_byte_is_a_refusal_not_an_exception — positional, path flag and free-text forms through parse_command, the issue's handle_text repro, and _confined directly.
• test_a_single_dash_token_is_refused_as_a_flag_not_taken_as_a_path.

CHANGELOG.md gets an entry per defect (Unreleased, newest-last), and the admission table in docs/cookbook/07-slack.md gets the no-repeat row.

Verification

Full pytest green and ruff check . clean, on the same worktree that ran green on untouched main first — no flaky failures showed up in either run, SIGALRM timing tests included.

Fixes #61
Fixes #64

🤖 Generated with Claude Code